• X
• Mastodon
• LinkedIn
• Facebook

tests/routers/openml/flows_test.py (1)

79-94: Good validation coverage; consider adding a test for URI-unsafe characters.

The empty field validation test is well-structured. However, since the primary motivation for this PR is to support flow names with URI-unsafe characters (e.g., sklearn.ensemble.AdaBoostClassifier(base_estimator=sklearn.tree.DecisionTreeClassifier)), consider adding a test case that explicitly validates this scenario works correctly with the new POST endpoint.

💡 Suggested test case for URI-unsafe flow names

def test_flow_exists_with_uri_unsafe_characters(py_api: TestClient, mocker: MockerFixture) -> None:
    """Verify POST endpoint handles flow names with URI-unsafe characters."""
    unsafe_name = "sklearn.ensemble.AdaBoostClassifier(base_estimator=sklearn.tree.DecisionTreeClassifier)"
    # This would fail with the old GET endpoint due to URI encoding issues
    response = py_api.post(
        "/flows/exists",
        json={"name": unsafe_name, "external_version": "1.0"}
    )
    # Expecting 404 since this flow doesn't exist in test data
    assert response.status_code == HTTPStatus.NOT_FOUND

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/routers/openml/flows_test.py` around lines 79 - 94, Add a new test
function (e.g., test_flow_exists_with_uri_unsafe_characters) that posts to the
"/flows/exists" endpoint using py_api with a URI-unsafe flow name such as
"sklearn.ensemble.AdaBoostClassifier(base_estimator=sklearn.tree.DecisionTreeClassifier)"
and external_version "1.0"; assert the response.status_code is
HTTPStatus.NOT_FOUND (or the expected non-existent-flow status) to verify the
POST endpoint accepts URI-unsafe characters in the request body rather than
failing due to encoding.

tests/routers/openml/flows_test.py (1)

75-109: Consider adding tests for URI-unsafe characters and the deprecated GET endpoint.

Given the PR's primary objective is to support URI-unsafe characters (e.g., sklearn flows with parentheses like sklearn.pipeline.Pipeline(...)), adding an explicit integration test with such characters would validate the fix end-to-end.

Additionally, there's no test coverage for the deprecated GET /flows/exists/{name}/{external_version} endpoint to verify its delegation behavior works correctly.

📝 Suggested test additions

# Test for URI-unsafe characters in POST (validates the fix)
async def test_flow_exists_with_uri_unsafe_chars(flow: Flow, py_api: httpx.AsyncClient) -> None:
    """Verify that flow names with URI-unsafe characters work via POST."""
    # This test would need a fixture flow with parentheses in its name
    name_with_parens = "sklearn.pipeline.Pipeline(steps=...)"
    response = await py_api.post(
        "/flows/exists", json={"name": name_with_parens, "external_version": "1.0"}
    )
    # Expecting 404 since the flow doesn't exist, but importantly NOT a URI encoding error
    assert response.status_code == HTTPStatus.NOT_FOUND


# Test for deprecated GET endpoint delegation
async def test_flow_exists_get_deprecated(flow: Flow, py_api: httpx.AsyncClient) -> None:
    """Verify deprecated GET endpoint still works and delegates correctly."""
    response = await py_api.get(f"/flows/exists/{flow.name}/{flow.external_version}")
    assert response.status_code == HTTPStatus.OK
    assert response.json() == {"flow_id": flow.id}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/routers/openml/flows_test.py` around lines 75 - 109, Add integration
tests covering URI-unsafe characters and the deprecated GET endpoint: add a test
similar to test_flow_exists_with_uri_unsafe_chars that posts to "/flows/exists"
with a name containing parentheses (e.g., "sklearn.pipeline.Pipeline(...)") and
asserts it returns HTTPStatus.NOT_FOUND (ensuring no URI-encoding failure), and
add a test like test_flow_exists_get_deprecated that calls GET
"/flows/exists/{name}/{external_version}" for an existing Flow fixture and
asserts HTTPStatus.OK and the same JSON payload as test_flow_exists ({"flow_id":
flow.id}) to verify delegation; place these alongside the existing tests
(test_flow_exists, test_flow_exists_not_exists,
test_flow_exists_rejects_empty_fields) so they run in the same suite.